Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TASK] Extend MathExpressionNode tests with chained math #398

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

NamelessCoder
Copy link
Contributor

Adds test cases for expressions like {a + 1 * 2}.

Also labels test cases.

@NamelessCoder
Copy link
Contributor Author

This would actually have been rebased away since it got fixed in another PR ;)

@mbrodala
Copy link
Contributor

Hm, so I just wasted time coming up with nice labels? ;-)

@NamelessCoder
Copy link
Contributor Author

Not sure it's wasted, I went the way of more or less writing out the entire fluid expression for easier debugging: https://github.com/TYPO3/Fluid/pull/396/files#diff-273f993b0da6a2bc91976237ec4881b6R45 - not sure which one to prefer. The only argument I can give is you wouldn't need to look at the test data set to know the expression that failed (without turning words into mental code first).

@mbrodala
Copy link
Contributor

OK, that's true indeed. Notice that I would rewrite $a(value) to $a (value) to make it clear that this is not a function call but instead a variable with its internal value.

@NamelessCoder
Copy link
Contributor Author

@mbrodala I think this is ready - trimmed down to only adding test cases for chained math.

@coveralls
Copy link

coveralls commented Mar 25, 2019

Pull Request Test Coverage Report for Build 1167

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.389%

Totals Coverage Status
Change from base Build 1158: 0.0%
Covered Lines: 2611
Relevant Lines: 2681

💛 - Coveralls

'Invalid operator returns zero' => ['1 gabbagabbahey 1', [], 0],
'Simple addition' => ['1 + 1', [], 2],
'Chained addition' => ['1 + 1 + 1', [], 3],
'Multiplication before addition' => ['1 + 1 * 2', [], 4],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, shouldn't this be 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expressions don't know about operator priority, so 1 + 1 = 2 * 2 = 4 is what it sees. It intentionally also doesn't know about parenthesis grouping in order to KISS.

If you know a reasonable way to implement operator priority (see also source of the expression class) I'd definitely consider adding that as a feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants